Skip to content
This repository was archived by the owner on Aug 29, 2023. It is now read-only.

Conversation

@masih
Copy link
Contributor

@masih masih commented Nov 8, 2022

Introduce an option to configure go-test to allow completely skipping 32-bit tests.

Fixes #388

@masih masih requested a review from a team as a code owner November 8, 2022 10:54
@masih masih requested a review from laurentsenta November 8, 2022 10:54
Copy link
Contributor

@marten-seemann marten-seemann left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should probably target the next branch, not master.

@masih
Copy link
Contributor Author

masih commented Nov 8, 2022

@marten-seemann based on a quick search, I can't find documentation on weather environment variables with boolean-looking value are interpreted as string or bool by GitHub Actions template. I would expect environment variable to be interpreted as string?

Considering the approach in this PR is consistent with the way by which other boolean config is parsed elsewhere (e.g. gogenerate config), would it make sense to move revising the boolean vs string interpretation outside of the scope of this PR?

@marten-seemann
Copy link
Contributor

I'm not really concerned about consistency here. @galargh might be though ;)
I'd recommend trying out this exact workflow in a branch in your repo, to make sure it actually works. I remember that the string vs. boolean mess has caused me some headache in the past.

@galargh
Copy link
Contributor

galargh commented Nov 8, 2022

@marten-seemann based on a quick search, I can't find documentation on weather environment variables with boolean-looking value are interpreted as string or bool by GitHub Actions template. I would expect environment variable to be interpreted as string?

That's correct. If it's in env, it's a string.

I'd recommend trying out this exact workflow in a branch in your repo, to make sure it actually works. I remember that the string vs. boolean mess has caused me some headache in the past.

Yes, please! Testing it out on a real repo would be extremely helpful.

This should probably target the next branch, not master.

Let's make this change too. We don't want to release it immediately to all the repos. If you want it to reach your repo earlier than the next major release, you can use technical preview feature - https://github.com/protocol/.github#technical-preview

@masih masih changed the base branch from master to next November 8, 2022 12:18
@masih masih force-pushed the masih/go_test_skip32bit branch from 273a162 to 6a78a03 Compare November 8, 2022 12:20
masih added a commit to ipni/storetheindex that referenced this pull request Nov 8, 2022
Test out a configuration to skip 32-bit go test via JSON configuration.

Relates to protocol/.github#412
@masih masih force-pushed the masih/go_test_skip32bit branch from 6a78a03 to 7f0e882 Compare November 8, 2022 12:23
masih added a commit to ipni/storetheindex that referenced this pull request Nov 8, 2022
Test out a configuration to skip 32-bit go test via JSON configuration.

Relates to protocol/.github#412
@masih masih force-pushed the masih/go_test_skip32bit branch from 7f0e882 to 5ab0e8c Compare November 8, 2022 12:25
Introduce an option to configure `go-test` to allow completely skipping
`32-bit` tests.

Fixes #388
@masih masih force-pushed the masih/go_test_skip32bit branch from 5ab0e8c to ccdf0fc Compare November 8, 2022 13:12
@masih
Copy link
Contributor Author

masih commented Nov 8, 2022

@galargh @marten-seemann the changes are tested here. please let me know if there is anything else in this PR you'd like changed.

Many thanks both for the speedy review 🚀

masih added a commit to ipni/storetheindex that referenced this pull request Nov 8, 2022
Test out a configuration to skip 32-bit go test via JSON configuration.

Relates to protocol/.github#412
masih added a commit to ipni/storetheindex that referenced this pull request Nov 9, 2022
Test out a configuration to skip 32-bit go test via JSON configuration.

Relates to protocol/.github#412
Copy link
Contributor

@galargh galargh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you 🙇 Let me know if you need help setting up Tech Preview for any of the repos.

@galargh galargh merged commit 4ef54da into next Nov 14, 2022
@galargh galargh deleted the masih/go_test_skip32bit branch November 14, 2022 08:21
galargh added a commit that referenced this pull request Feb 8, 2023
* Add option to skip `32-bit` go test (#412)

Introduce an option to configure `go-test` to allow completely skipping
`32-bit` tests.

Fixes #388

* Run at most 1 dispatch job per ref (#414)

* fix: check if git tag returns any results (#415)

* make go generate print the commands it executs (#440)

* use pull_request_target event for release-check workflow (#295)

* Revert "include cross-package coverage in codecov"

* Revert "Revert "include cross-package coverage in codecov""

* Make automerge a reusable workflow (#260)

* move automerge from template to workflows

* make automerge reusable and use it from new automerge template

* pass parent job name to reusable automerge

* check github actions yamls (#272)

* check github actions yamls

* make yaml linter happy about go-test

* mention VS Code YAML extension in the readme

* add info about other YAML checking extensions

* make yaml checker more generic

* use validate-yaml-schema action from mainline (#277)

* upgrade lewagon/wait-on-check-action to v1.1.1 (#278)

* always add a version.json file if it doesn't exist (#281)

* fix go-test runner string

* check if tag already exists in release-check (#287)

* allow specifying custom PATH for 386 arch (#289)

* use pull_request_target event for release-check workflow

* add comment on missing version.json

* chore: revert release checker path trigger change

* chore: add footnote when non-docs files are modified with the release

* fix: prev version calculation

* feat: allow configuring custom go-test runners (#443)

* feat: allow configuring custom go-test runners

* docs: update readme to include info on configuration variables

* feat: allow skipping go-test on certain OSes (#455)

* feat: standarise JSON config reading

* feat: allow skipping go-test on certain OSes

* fix: go-test conditional

* chore: show config after extracting it

* chore: udpate actions and go modules (#458)

* fix: source read-config from next for now

* simplify Go version upgrade procedure (#280)

* chore: simplify Go version upgrade procedure

* chore: add default for the go-version input of release-check

* Update .github/actions/copy-workflow-go/action.yml

* Update configs/README.md

Co-authored-by: Laurent Senta <[email protected]>

---------

Co-authored-by: Laurent Senta <[email protected]>

* Go through all the workflows and clean them up ahead of the next major release (#462)

* chore: clean up deprecated set-output

* chore: do not use substitution inside run

* chore: do not use substitution in if

* chore: skip env var brakets where possible

* fix: env var substitution

* fix: double toJSON

* Update templates/.github/workflows/js-test-and-release.yml

* feat: create gh releases in release-check/releaser workflows (#456)

* feat: create gh releases in release-check/releaser workflows

* fix: fill expr in release check workflow

* fix: add missing gh token in release check

* fix: add missing prev version env var in release check workflow

* fix: release check in release check

* chore: clean up obsolete step from releaser

* fix: step outputs in release workflows

* fix: labels in releaser

* fix: action gh release

* update go version to 1.20.x (#463)

* update go version to 1.20.x

* fix: go 1.20 upgrade

* Revert "fix: go 1.20 upgrade"

This reverts commit ceb72ef.

* clean up where source ref was set to next (#464)

* perform self-review before final release

---------

Co-authored-by: Masih H. Derkani <[email protected]>
Co-authored-by: Marten Seemann <[email protected]>
Co-authored-by: Laurent Senta <[email protected]>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Should we support opting out of 32bit tests in go-test workflow?

4 participants